Skip to content

fix(public/install.sh): fail early if req buildtools are missing#49

Merged
steipete merged 5 commits intoopenclaw:mainfrom
knocte:wip/checkBuildTools
Feb 13, 2026
Merged

fix(public/install.sh): fail early if req buildtools are missing#49
steipete merged 5 commits intoopenclaw:mainfrom
knocte:wip/checkBuildTools

Conversation

@knocte
Copy link
Contributor

@knocte knocte commented Feb 6, 2026

Would fix some of the issues some people have been reporting in: openclaw/openclaw#5462

[AI assisted via Codex 5.2]

Would fix some of the issues some people have been reporting in:
openclaw/openclaw#5462

[AI assisted via Codex 5.2]
@vercel
Copy link

vercel bot commented Feb 6, 2026

@knocte is attempting to deploy a commit to the Amantus Machina Team on Vercel.

A member of the Team first needs to authorize it.

local skip_onboard=false

# Fail early if required build tools are missing (before any npm commands)
check_build_tools
Copy link

@vercel vercel bot Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check_build_tools() is called too early before Homebrew installation on macOS, and unconditionally for npm installs which don't need build tools

Fix on Vercel

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check_build_tools() is called too early before Homebrew installation on macOS

Better to fail eariler than installing anything? AFAIU homebrew wouldn't install make or cmake

unconditionally for npm installs which don't need build tools

How would we know which npm installs wouldn't need make or cmake? AFAIU there is no way to know

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You make good points about early failure being preferable to cryptic npm errors, and you're right that npm installs might also need build tools depending on prebuilt binary availability.

However, there's still a practical issue with the current placement: On macOS systems without Xcode CLT, this check will fail and exit before install_homebrew runs. Since brew install make cmake can provide these tools, it would be better to let Homebrew be installed first.

The ideal flow would be:

  1. Install Homebrew (macOS only)
  2. Then check for build tools (with option to auto-install via brew on macOS)
  3. Then proceed with Node.js and openclaw installation

This way, users on macOS get a better experience - the installer can automatically provide the missing tools via Homebrew rather than just failing. Linux users still get the early clear error message.

The current approach works, but it's not as user-friendly as it could be on macOS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented follow-up on this PR.

What changed:

  • Removed the unconditional early check_build_tools gate (no pre-Homebrew hard stop).
  • Added npm-log signature detection for missing native toolchains.
  • Added auto-install/retry path for native build failures:
    • Linux: install build-essential python3 make g++ cmake (or distro equivalents).
    • macOS: ensure Xcode CLT + brew install cmake.
  • Kept existing ENOTEMPTY/EEXIST recovery behavior.

Refs:

  • public/install.sh:141 (signature detection)
  • public/install.sh:150 (Linux auto-install)
  • public/install.sh:178 (macOS auto-install)
  • public/install.sh:238 (retry flow in npm install)
  • public/install.sh:1291 (Homebrew runs before npm path; no early build-tools exit)

Tests:

  • Added installer unit coverage for detection + auto-install retry:
    • scripts/test-install-sh-unit.sh:130
    • scripts/test-install-sh-unit.sh:153
  • Also fixed stale unit references from clawdbot to openclaw in this branch so the unit script executes.

@steipete steipete merged commit 6c4edfd into openclaw:main Feb 13, 2026
0 of 2 checks passed
@steipete
Copy link
Contributor

Landed via temp rebase onto main.

  • Gate: bash -n public/install.sh && bash -n scripts/test-install-sh-unit.sh && bash scripts/test-install-sh-unit.sh && pnpm build
  • Land commit: 7c74d0a
  • Merge commit: 6c4edfd

Thanks @knocte!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants